-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: no-method-error with Grape::Entity < 0.5.0 #9
base: master
Are you sure you want to change the base?
Conversation
Thank you for the PR. Can you provide a spec? I am not using Grape and the previous introduction of this feature was not tested. I also updated the ruby version(s) for Travis on master. Can you rebase so we can run the build? |
@brlo just pinging since I didn't mention you in the original comment. |
No problem, but need a few days more. |
Thanks a lot! |
entity.represent({ping: :pong}).to_json | ||
end | ||
get '/ping?rack-profiler' | ||
expect(JSON.parse(last_response.body)['response']['body']).to eql(sample_hash.to_json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [92/80]
subject.get '/ping' do | ||
entity.represent({ping: :pong}).to_json | ||
end | ||
get '/ping?rack-profiler' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
entity = Class.new(Grape::Entity) { expose :ping } | ||
|
||
subject.get '/ping' do | ||
entity.represent({ping: :pong}).to_json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant curly braces around a hash parameter.
Space inside { missing.
Space inside } missing.
it 'working with Grape::Entity' do | ||
entity = Class.new(Grape::Entity) { expose :ping } | ||
|
||
subject.get '/ping' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
expect(JSON.parse(last_response.body)['response']['body']).to eql(sample_hash.to_json) | ||
end | ||
|
||
it 'working with Grape::Entity' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
present Object.new, with: entity_mock | ||
end | ||
get '/ping?rack-profiler' | ||
expect(JSON.parse(last_response.body)['response']['body']).to eql(sample_hash.to_json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [92/80]
subject.get '/ping' do | ||
present Object.new, with: entity_mock | ||
end | ||
get '/ping?rack-profiler' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
entity_mock = Object.new | ||
allow(entity_mock).to receive(:represent).and_return(sample_hash.to_json) | ||
|
||
subject.get '/ping' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
expect(JSON.parse(last_response.body)['response']['body']).to eql('') | ||
end | ||
|
||
it 'working with #present' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
nil | ||
end | ||
get '/ping?rack-profiler' | ||
expect(JSON.parse(last_response.body)['response']['body']).to eql('') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
subject.get '/ping' do | ||
nil | ||
end | ||
get '/ping?rack-profiler' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
end | ||
|
||
it 'working with NULL' do | ||
subject.get '/ping' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
expect(JSON.parse(last_response.body)['response']['body']).to eql('pong') | ||
end | ||
|
||
it 'working with NULL' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
'pong' | ||
end | ||
get '/ping?rack-profiler' | ||
expect(JSON.parse(last_response.body)['response']['body']).to eql('pong') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
subject.get '/ping' do | ||
'pong' | ||
end | ||
get '/ping?rack-profiler' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
||
it 'working with String' do | ||
subject.get '/ping' do | ||
'pong' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
describe Rack::Grape::EndpointJson do | ||
include Rack::Test::Methods | ||
subject { Class.new(Grape::API) } | ||
let(:sample_hash){ {ping: :pong} } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space missing to the left of {.
Space inside { missing.
Space inside } missing.
@@ -0,0 +1,59 @@ | |||
require "json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing frozen string literal comment.
@@ -38,7 +38,8 @@ def initialize(app, &block) | |||
subscribe_to_default | |||
block.call(self) if block_given? | |||
|
|||
if defined?(::Grape::Endpoint) | |||
# This patch is required because of bug with Grape-Entity which is fixed in version 0.5.0 | |||
if defined?(::Grape::Endpoint) && defined?(GrapeEntity::VERSION) && (Gem::Version.new(GrapeEntity::VERSION) < Gem::Version.new("0.5.0")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [142/80]
@@ -38,7 +38,8 @@ def initialize(app, &block) | |||
subscribe_to_default | |||
block.call(self) if block_given? | |||
|
|||
if defined?(::Grape::Endpoint) | |||
# This patch is required because of bug with Grape-Entity which is fixed in version 0.5.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [95/80]
if defined?(::Grape::Endpoint) | ||
# This patch is required because of bug with Grape-Entity | ||
# which is fixed in version 0.5.0 | ||
if (defined?(::Grape::Endpoint) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use parentheses around the condition of an if.
The bug appears only if you are using library grape-entity version less than 0.5.0. In this case, Body can be anything and #to_hash not work in most cases. You can check it in https://github.com/ruby-grape/grape-on-rack |
There is no method 'to_hash' for Grape::Entity instance. Just call 'as_json' directly.
https://github.com/ruby-grape/grape-entity/blob/07bdc590754a35750a17554beedf3b8025fc4c08/lib/grape_entity/entity.rb#L488